tools/js2c-inputs.xcfilelist (1)

1-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

New builtin .js files must be added here too, or incremental builds silently skip regeneration.

This file drives Xcode's incremental-build change detection for the "Generate RuntimeBuiltins" phase, but the phase's shell script actually globs NativeScript/runtime/js/*.js at runtime. If a contributor adds a new builtin script without updating this list, Xcode won't detect it as a changed input and may skip re-running js2c.mjs, leaving the new file out of the generated RuntimeBuiltins.{h,cpp} until an unrelated input changes or a clean build happens.

Consider adding a short comment here (or in tools/js2c.mjs's usage text) noting that this list must be kept in sync with NativeScript/runtime/js/*.js.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/js2c-inputs.xcfilelist` around lines 1 - 12, Add a concise maintenance
comment to the input list documenting that every new
NativeScript/runtime/js/*.js builtin must also be added here so Xcode
incremental builds rerun js2c.mjs. Keep the existing file entries and generation
behavior unchanged.

NativeScript/runtime/js/require-factory.js (1)

1-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: tidy the formatting carried over from the C++ string literal.

The stray line break inside the if body (Lines 4-5) and trailing whitespace are artifacts of the old embedded literal. Now that this is a real source file, normalizing it costs nothing and keeps future diffs readable. Semantics unchanged.

♻️ Proposed cleanup

-(function() { 
-    function require_factory(requireInternal, dirName) { 
-        return function require(modulePath) { 
-            if(global.__pauseOnNextRequire) {  debugger; 
-global.__pauseOnNextRequire = false; }
-            return requireInternal(modulePath, dirName); 
-        } 
-    } 
-    return require_factory; 
-})()
+(function () {
+  function require_factory(requireInternal, dirName) {
+    return function require(modulePath) {
+      if (global.__pauseOnNextRequire) {
+        debugger;
+        global.__pauseOnNextRequire = false;
+      }
+      return requireInternal(modulePath, dirName);
+    };
+  }
+  return require_factory;
+})();

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@NativeScript/runtime/js/require-factory.js` around lines 1 - 10, Normalize
formatting in the require_factory wrapper, especially the __pauseOnNextRequire
conditional, by removing the embedded line break and trailing whitespace while
preserving debugger invocation, flag reset, and requireInternal behavior.

NativeScript/runtime/js/smart-stringify.js (1)

3-13: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Optional: seen as an array yields O(n²) scans and false [Circular] hits for shared (non-cyclic) references.

seen.indexOf(value) is linear per visited object, and entries are never popped when leaving a subtree, so a repeated-but-acyclic reference is reported as circular. Swapping to a Set fixes the complexity; fixing the false positives needs an ancestor stack instead. Both change existing output, so this is fine to defer given the PR's extraction-fidelity goal.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@NativeScript/runtime/js/smart-stringify.js` around lines 3 - 13, Defer
changes to the circular-reference tracking in the replacer function: retain the
existing seen array and output behavior for this extraction-focused change. Do
not alter complexity or shared-reference handling unless a follow-up explicitly
scopes those behavioral changes.

NativeScript/runtime/Helpers.mm (1)

497-503: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider a TryCatch around the builtin run.

If RunBuiltin fails (compile/run error), the scheduled exception stays pending on the isolate while this function returns an empty handle. JsonStringifyObject then continues into JSON::Stringify, so the failure surfaces at an unrelated site. Wrapping this call in a local TryCatch (and logging via tns::LogError) keeps the failure contained, matching the pattern used elsewhere in this file.

♻️ Proposed refactor

   Local<Value> result;
-  bool success = BuiltinLoader::RunBuiltin(context, BuiltinId::kSmartStringify).ToLocal(&result);
-  tns::Assert(success, isolate);
-
-  if (result.IsEmpty() || !result->IsFunction()) {
+  TryCatch tc(isolate);
+  bool success = BuiltinLoader::RunBuiltin(context, BuiltinId::kSmartStringify).ToLocal(&result);
+  if (!success || tc.HasCaught()) {
+    tns::LogError(isolate, tc);
+    return Local<v8::Function>();
+  }
+
+  if (result.IsEmpty() || !result->IsFunction()) {
     return Local<v8::Function>();
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@NativeScript/runtime/Helpers.mm` around lines 497 - 503, In the
builtin-loading flow around BuiltinLoader::RunBuiltin, add a local v8::TryCatch
and handle compile/run failures before returning an empty function handle. Log
the caught exception through tns::LogError, clear or contain the pending isolate
exception, and preserve the existing result.IsEmpty()/IsFunction() validation
for successful execution.

NativeScript/runtime/WeakRef.cpp (1)

10-19: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider capturing exception details before asserting, like TSHelpers::Init.

Unlike TSHelpers::Init (which wraps the call in a TryCatch and calls tns::LogError(isolate, tc) before asserting), this path asserts on bare success with no way to see why the WeakRef builtin failed to compile/run. Since WeakRef is treated as similarly critical (hard Assert), aligning the diagnostics would make a future failure easier to debug.

♻️ Proposed alignment with TSHelpers::Init

 void WeakRef::Init(Local<Context> context) {
   Isolate* isolate = v8::Isolate::GetCurrent();
+  TryCatch tc(isolate);

   Local<Value> result;
-  bool success =
-      BuiltinLoader::RunBuiltin(context, BuiltinId::kWeakRef).ToLocal(&result);
-  tns::Assert(success, isolate);
+  if (!BuiltinLoader::RunBuiltin(context, BuiltinId::kWeakRef).ToLocal(&result) &&
+      tc.HasCaught()) {
+    tns::LogError(isolate, tc);
+  }
+  tns::Assert(!result.IsEmpty(), isolate);
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@NativeScript/runtime/WeakRef.cpp` around lines 10 - 19, Update WeakRef::Init
to wrap BuiltinLoader::RunBuiltin in a v8::TryCatch, log the captured exception
with tns::LogError(isolate, tc) when execution fails, then retain the existing
tns::Assert(success, isolate) behavior.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes